[TV] Make the app a first-class media-session citizen - #5740

Claude finished @sztomek's task in 5m 41s —— View job

Code review: TV media-session wiring

Read the diff against origin/main and traced the runtime paths it activates (PlaybackServiceToggle, FeatureFlag, MediaSessionManager, PackageValidator, NotificationHelperImpl), comparing against how wear and app do the same thing.

• Read the PR diff
• Compare with wear/app manifests
• Trace PlaybackServiceToggle / NotificationHelper / MediaSessionManager behaviour on TV
• Check caller validation, launch-intent and foreground-service implications
• Post review

Overall: the approach is right — reusing the shared services through the runtime toggle instead of adding TV-specific plumbing, and the manifest block is a faithful copy of wear/app. One finding is worth acting on before merge; the rest are minor. No security or performance concerns of substance.

1. FeatureFlag is never initialised on TV — the toggle always uses the compile-time default (TvApplication.kt:37)

PlaybackServiceToggle reads FeatureFlag.isEnabled(Feature.MEDIA3_SESSION), and FeatureFlag.isEnabled returns feature.defaultValue when no providers are registered. FeatureFlag.initialize(...) is only called from AppLifecycleObserver.setup() and AutomotiveApplication; TvApplication calls neither, and there is no FeatureFlag/FeatureProvider reference anywhere under tv/src. Since MEDIA3_SESSION.defaultValue = isDebugOrPrototypeBuild:

• debug/prototype TV → media3 PlaybackService (what the emulator run verified),
• release TV → LegacyPlaybackService, with the Firebase remote flag and dev toggle having no effect on TV.

It is at least self-consistent — MediaSessionManager.useMedia3Session reads the same uninitialised flag, so the enabled component and the session type can't disagree — but the path that ships to release users is the untested one. Wear calls appLifecycleObserver.setup() (which initialises the flags) immediately before ensureCorrectServiceEnabled; TV should either do the same or explicitly document that it ships the default, and verify a prototype build. Whichever way, any future flag init must go before line 37, otherwise the toggle and MediaSessionManager's lazy read can diverge for that launch. Details in the inline comment.

2. Permissions block — minor (inline)

• POST_NOTIFICATIONS is already merged in from modules/services/repositories/src/main/AndroidManifest.xml:5; repeating it is harmless but it isn't what unblocks anything. Nothing in tv/src requests it at runtime, so on API 33+ the FGS notification is posted but not displayed (the service and session still work).
• The FOREGROUND_SERVICE_DATA_SYNC gap is a bit sharper than described: the same library manifest merges in SystemForegroundService with foregroundServiceType="dataSync", so the APK declares a dataSync FGS with no matching permission. DownloadEpisodeWorker catches the failure; FixDownloadsWorker / OpmlImportTask don't. TV triggers none of them today, so deferring is fine.

3. Main-thread startup work — nit

setupNotificationChannels() creates 13 channels and ensureCorrectServiceEnabled() makes 2–4 synchronous PackageManager binder calls, both on the main thread in onCreate(). The toggle is guarded by a getComponentEnabledSetting read so it only writes when the state actually changes, and this matches wear exactly — but the channel setup could move into the existing applicationScope.launch if TV startup latency ever matters. The toggle itself must stay synchronous, so it's correctly placed.

Things I checked that are fine

• Caller validation on TV. Exporting the browse service is required and safe here: PackageValidator.isKnownCaller admits system-signed callers, Process.SYSTEM_UID, and holders of MEDIA_CONTENT_CONTROL, which covers the Android TV system/Assistant surfaces even though allowed_media_browser_callers.xml lists no TV packages. The Android 11+ <queries> list isn't a problem either — binding grants mutual package visibility.
• Leanback launch intent. Context.getLaunchActivityPendingIntent() already falls back to getLeanbackLaunchIntentForPackage, so the session activity / notification content intent resolves on TV.
• Ordering. The toggle runs before playbackManager.setup(), which is what matters for startServiceIfNeeded() resolving a component.
• Hilt. NotificationHelper binding is satisfied for the TV component (compile-time verified), and PlaybackService's deps were already validated in the TV graph before this PR.
• Omitting MediaButtonReceiver matches wear and is correctly reasoned — it only forwards external ACTION_MEDIA_BUTTON broadcasts.

Adjacent, not introduced here

Util.getAppPlatform() has no TV branch, so TV resolves to AppPlatform.Phone. In PlaybackManager.sendDataWarningNotification (PlaybackManager.kt:2377-2385), getLaunchIntentForPackage returns null on a leanback-only manifest, the Phone && intent != null guard fails, and the else branch calls PendingIntent.getActivity(..., null, ...). Same class of bug the Context.getLaunchActivityPendingIntent() leanback fallback already fixed. Only reachable when streaming on a connection the system reports as metered, so it's unlikely on TV and clearly out of scope — but now that TV playback is first-class it's worth a follow-up.

No changes were pushed — this is review feedback only.
• branch feat/tv-media-session

Tested this branch (4bbda40) on real hardware — a Philips 4K A1 (Android 11),installDebugProd . Results for the items listed as not-yet-verified:

Startup and component state ✅
App launches cleanly, no crash from the new onCreate() wiring.
enabledComponents → ...repositories.playback.PlaybackService
disabledComponents → ...repositories.playback.LegacyPlaybackService

Media session ✅
dumpsys media_session shows androidx.media3.session.id.PocketCastsMedia3Session, and the system designates it as the media button session (Media button session is au.com.shiftyjelly.pocketcasts.debug/...PocketCastsMedia3Session).

Background continuation ✅
Playing an episode and pressing Home: audio continues uninterrupted.

Remote media-key control ✅
input keyevent 85 (KEYCODE_MEDIA_PLAY_PAUSE) toggles pause/resume with the app in the background.

Screen-off — behaves differently to a phone, and I don't think it's a defect
input keyevent 223 (KEYCODE_SLEEP) puts the TV into standby and audio stops. But the app process and the media session both survive, and a single media-key press after wake resumes playback immediately. That reads as TV standby suspending playback rather than the session being lost — but you'd know better than me whether that's the intended outcome on this hardware.

Not tested: media resumption after reboot, Assistant voice control, and the release/legacy path — this was a debug build, so per the review comment MEDIA3_SESSION resolved to its isDebugOrPrototypeBuild default and only the media3 service was exercised. Happy to run a prototype build against LegacyPlaybackService if that's useful.

One incidental note: the app installed under user User 10 on this device rather than the owner profile. Didn't seem to affect anything, just flagging it.

While testing this branch I noticed there's still no way to control playback without leaving whatever you're browsing. My closed #5737 had a mini-player drawer on the remote's Options/MENU key — transport controls slide in over the current screen, Menu or Back dismisses. I've since rebased it onto current main and cut it down to reuse TvSeekBar and the existing tv_nothing_playing_* strings rather than the hand-rolled controls it originally had. It's one new file plus a small hook in TvScaffold. Is that something you'd want? Happy to open it as its own PR, or drop it if you have designs for this already.

hi @nolengreenspan

Sorry for the long wait, i was AFK last week.
Thank you for having a look and actually testing this branch on your end. 🙏

I noticed there's still no way to control playback without leaving whatever you're browsing. My closed #5737 had a mini-player drawer on the remote's Options/MENU key — transport controls slide in over the current screen, Menu or Back dismisses. I've since rebased it onto current main and cut it down to reuse TvSeekBar and the existing tv_nothing_playing_* strings rather than the hand-rolled controls it originally had. It's one new file plus a small hook in TvScaffold. Is that something you'd want? Happy to open it as its own PR, or drop it if you have designs for this already.

I believe this piece would require input from our design team, but I'll ask them to investigate how a miniplayer would work best - if they approve the idea and draft a deisng for it, a follow-up PR will be quite likely.

Claude finished @sztomek's task in 5m 25s —— View job

Code review: TV media-session wiring (round 2)

Re-reviewed after 775a56f ("Address PR comments"), tracing the paths this now activates on TV: FeatureFlag / the three providers, PlaybackServiceToggle, NotificationHelperImpl, PlaybackService / LegacyPlaybackService, and the notification surfaces setupNotificationChannels() newly unblocks.

• Read the updated diff
• Verify the new setupFeatureFlags() against AppLifecycleObserver / AutomotiveApplication / wear
• Check BuildConfig.IS_PROTOTYPE for the tv module
• Re-check manifest, permissions, and startup ordering
• Post review

Round-1 blocker is resolved. setupFeatureFlags() runs before PlaybackServiceToggle.ensureCorrectServiceEnabled() (TvApplication.kt:48-50), so the toggle and MediaSessionManager.useMedia3Session now read the same initialised provider set, and remote config / dev toggles apply on TV. The provider list matches AppLifecycleObserver.kt:106-117 exactly, and the ordering matches wear (PocketCastsWearApplication.kt:109-111). Nothing blocking left; three notes below.

1. prototype isn't a buildable variant for :tv — release is the only untested path (inline)

tv/build.gradle.kts:42-46 does builder.enable = builder.buildType != "prototype", so BuildConfig.IS_PROTOTYPE is always false here and that arm of the if is dead. TV has two flag paths, not three: debug/debugProd → PreferencesFeatureProvider, release → Firebase + DefaultReleaseFeatureProvider.

That matters for verification: a prototype build isn't available as the "test the release path" shortcut. And the release path is deterministic in a way worth stating — Feature.MEDIA3_SESSION.defaultValue is false in release, and FirebaseConfig.kt:29-31 seeds media3_session into setDefaultsAsync from that same default, so a fresh release install enables LegacyPlaybackService. FirebaseRemoteFeatureProvider.init fetches asynchronously and resolves long after onCreate, so a remote flip only takes effect on the next launch; LegacyPlaybackService.onCreate's mediaSession == null → stopSelf() guard covers the launch in between. All of this is identical to app, so nothing is introduced here — but release TV ships legacy MediaSessionCompat, and neither the emulator run nor @nolengreenspan's Philips A1 run (both debug) exercised it.

2. setupFeatureFlags() is now the third verbatim copy — minor

Byte-identical to AppLifecycleObserver.kt:106-117 and AutomotiveApplication.kt:157-168. TV deliberately skips AppLifecycleObserver, so the duplication is understandable, but a shared helper in :modules:services:utils would keep the provider list and its "must run before anything reads a flag" constraint in one place. Fine to defer. Fix this →

3. Creating the channels re-opens two notification surfaces on TV that were previously dropped — low severity

Before this PR the TV app had no notification channels, so any NotificationManagerCompat.notify() on API 26+ was silently dropped by the system. setupNotificationChannels() changes that, and two playback notifications become postable on TV:

• PlaybackManager.sendDataWarningNotification (PlaybackManager.kt:2374) and the playback-error notification (PlaybackManager.kt:2429/2435).
• Their action buttons go through buildNotificationIntent (PlaybackManager.kt:2445), which targets NotificationBroadcastReceiver — declared only in app/src/main/AndroidManifest.xml:586, not in the repositories library manifest. On TV the receiver doesn't exist, so those buttons are no-ops.
• sendDataWarningNotification also still hits the AppPlatform.Phone && intent != null guard at PlaybackManager.kt:2380 that round 1 flagged; getLaunchIntentForPackage is null on a leanback-only manifest, so it falls to PendingIntent.getActivity(..., null, ...).

Mitigations: same situation on wear/automotive (pre-existing, not introduced here), reachable only when streaming on a connection the system reports as metered, and nothing in tv/src requests POST_NOTIFICATIONS at runtime, so on API 33+ none of it is displayed anyway. Genuinely a follow-up, not a change to this PR — just no longer purely theoretical now that the channels exist.

Verified fine

• Ordering. Flags → channels → toggle → playbackManager.setup(). No production code reads a flag with immutable = true (only FeatureFlagTest), so nothing constructed during Hilt field injection in super.onCreate() can snapshot a pre-initialize() value and lock it in.
• FeatureFlag.initialize non-idempotency. It does providers.addAll(...) without dedup, but Application.onCreate runs once per process and PlaybackService shares that process — no double registration.
• No FirebaseApp.initializeApp(this) on TV (app and wear both call it defensively). Not a regression: SettingsImpl already injects FirebaseRemoteConfig, so FirebaseRemoteConfig.getInstance() was already reached on every TV launch before this PR.
• Manifest. Service blocks are byte-equivalent to wear/src/main/AndroidManifest.xml:80-102 and app/src/main/AndroidManifest.xml:605-626, including the androidx.media3.session.MediaLibraryService action needed for media3 and the android.media.browse.MediaBrowserService action that makes media resumption resolvable. enabled="false" on both is correct — the toggle owns the choice.
• google-services.json. tv/src/debug and tv/src/debugProd both have one; release is injected by CI, same as app/wear/automotive.
• Omitting MediaButtonReceiver. Matches wear; the reasoning holds. Note @nolengreenspan's input keyevent 85 test goes via AudioManager.dispatchMediaKeyEvent to the active session, so it confirms the session path, not the broadcast path — but the broadcast path is the one deliberately out of scope.
• getComponentEnabledSetting guard in PlaybackServiceToggle.kt:29-30 means no PackageManager write on the steady-state launch.

On the screen-off observation

@nolengreenspan's KEYCODE_SLEEP result reads as expected rather than a defect — Android TV standby suspends the audio output, but the process and session surviving with an immediate resume on the next key press is exactly what a healthy foreground service + session looks like. The wired-up session is what makes that resume work at all.

No changes pushed — review feedback only. I did not run ./gradlew spotlessCheck or a build; the diff is formatted consistently with the surrounding files and CI covers both.
• branch feat/tv-media-session

@nolengreenspan

I've synced with the design team on the miniplayer, and they decided not to explore the idea further for now. Our current focus is to ship the TV app for android and keep it on par with its appleTV counterpart (that's been already released).
This doesn't mean that we won't add this feature in the future, so stay tuned for updates.
Thank you for your contribution and ideas, we truly appreciate your support and engagement with our product.

Internal comms for reference: p1787590386685359-slack-C0ATWH7BNH3
Miniplayer prototype PR: #5765

My pleasure thank you

Claude finished @sztomek's task in 5m 58s —— View job

Code review: TV media-session wiring (round 3)

Reviewed the two new commits since round 2 — 4e43847 (skip the session activity on TV) and 650cb22 (open the Now Playing tab from the media card) — and traced the paths they touch: MediaSessionManager session construction, Util.isTv, getLaunchActivityPendingIntent, the TvActivity intent handling, TvLaunchRequests, and the TvScaffold / TvScaffoldViewModel tab selection.

• Read the new diff
• Trace getLaunchActivityPendingIntent / session-activity fallback on TV
• Check the media-card launch-intent detection in TvActivity
• Review TvLaunchRequests channel semantics and singleTask
• Check cross-platform impact of the MediaSessionManager change
• Post review

One blocker: the new ViewModel parameter breaks the existing unit test's compilation. Everything else is minor or a design suggestion. The round-1/round-2 findings remain resolved, and Util.isTv reading the pocketcasts_tv manifest meta-data means the shared-code change is genuinely TV-module-only — no risk to app/wear/automotive, and no risk of the phone APK sideloaded on a TV device taking the new branch.

1. 🔴 :tv:testDebugUnitTest will not compile (inline)

launchRequests: TvLaunchRequests is inserted as the third positional parameter of TvScaffoldViewModel (TvScaffoldViewModel.kt:24), but tv/src/test/java/au/com/shiftyjelly/pocketcasts/TvScaffoldViewModelTest.kt:55 still does:

TvScaffoldViewModel(syncManager, signOutManager, upNextQueue)

upNextQueue now lands on the TvLaunchRequests slot → type mismatch plus a missing argument. TvLaunchRequests has an @Inject no-arg constructor, so the fix is a real instance rather than a mock — and that gives a natural home for a test of the new wiring (requestOpenNowPlaying() → uiState.selectedTab == TvTab.NowPlaying), which is currently untested.

2. onCreate re-fires the open request on activity recreation (inline)

getIntent() keeps returning the original launch intent, so any recreation — config change (locale, display size / resolution, night mode) or process-death restore — re-runs handleLaunchIntent(intent) on the same media-card intent and pulls the user back to Now Playing from wherever they had navigated. Needs a savedInstanceState == null guard; onNewIntent should also setIntent(intent), which is the conventional companion for a singleTask activity.

3. The media-card detection relies on undocumented launcher behaviour (inline)

ACTION_MAIN && CATEGORY_LAUNCHER && !CATEGORY_LEANBACK_LAUNCHER can only match because system UI builds an explicit-component intent and tags it CATEGORY_LAUNCHER — the manifest filter itself declares MAIN + LEANBACK_LAUNCHER only. That's an observation, not a contract: if a Google TV/OEM launcher revision switches to the leanback category the auto-open silently stops with no signal, and unrelated MAIN/LAUNCHER entry points are false positives.

Since 4e43847 gave up setSessionActivity on TV specifically so this fallback intent is the one used, the cleaner inversion is to keep the session activity and let it carry your own marker — that's what the API is for. getLaunchActivityPendingIntent() already resolves TvActivity through its leanback fallback, so an optional extras: Bundle? parameter there needs no :repositories → :tv reference, and TvActivity then reads an explicit boolean extra.

4. Dropping sessionActivity on TV — two consequences worth confirming (inline)

• The root cause isn't pinned down and two things changed together. singleTask is what makes onNewIntent fire at all (a standard root activity never receives it) and it also changes how an incoming MAIN intent routes to an existing task. It's plausible the launch-mode change alone fixed Open — worth one A/B on the device that reproduced it (singleTask + session activity restored) before permanently special-casing shared code.
• sessionActivity is also media3's notification content intent (DefaultMediaNotificationProvider), so the TV playback FGS notification becomes non-actionable. Low impact — TV surfaces the card, not the shade, and nothing in tv/src requests POST_NOTIFICATIONS at runtime so it isn't displayed on API 33+ anyway. Please also confirm the card still appears with no session activity set, since some surfaces gate their affordances on it.

5. Buffered request can fire much later than the launch — minor

TvLaunchRequests is a @Singleton holding a Channel(CONFLATED) and TvScaffold is a nav destination, so the request is only consumed once TvOnboardingRoutes.HOME is composed. If the launch lands while the user is on Landing / Sign-In / Syncing (possible — TV supports "continue without account", so playback and a media card can exist while signed out), and while HOME is off-screen in the back stack its LaunchedEffect is disposed, the element sits in the buffer and fires whenever the scaffold next appears — potentially minutes later, reading as a spurious jump to Now Playing. Consider dropping requests older than the current launch, or consuming them in viewModelScope so the lifetime is tied to the scaffold's ViewModel rather than to its composition.

6. An open request while the queue is empty leaves the tab stuck — minor

TvScaffoldViewModel.init corrects a stale Now Playing selection only on a hasCurrentEpisode transition. hasCurrentEpisode starts false, so if the open request arrives and the queue then resolves to a non-Loaded state, the collector never re-emits and the tab stays selected on TvNowPlayingUiState.Empty. It degrades gracefully (the tv_nothing_playing_* empty state, and the user can navigate away) and needs a stale media card to reach, but a combine(selectedTab, hasCurrentEpisode) — or gating openNowPlaying() on hasCurrentEpisode.value — would close it.

Verified fine

• Util.isTv is manifest-meta-data based (pocketcasts_tv), not a UI-mode check, so the MediaSessionManager change cannot affect app/wear/automotive, including the phone APK sideloaded on a TV.
• Ordering in TvActivity.onCreate. handleLaunchIntent runs after super.onCreate() (so @Inject lateinit var launchRequests is initialised) and before setContent; the CONFLATED buffer holds the request until the scaffold composes, so there's no cold-start race.
• launchMode="singleTask" on a single-activity app is effectively singleTop — no back-stack clearing to worry about — and is required for onNewIntent delivery. Reasonable.
• LaunchedEffect(viewModel) is keyed on a stable value and openNowPlaying is remember(viewModel), so no stale-lambda capture; a recreation re-collects from an empty conflated channel and fires nothing spurious.
• Hilt. TvLaunchRequests is @Singleton with an @Inject constructor — no module needed, injectable into both the activity and the ViewModel.
• receiveAsFlow single-consumer semantics are fine here: the scaffold is the only collector, and sequential collectors don't lose buffered elements.
• Round 2's items are unchanged and still fine: flag init before the toggle, manifest service blocks, enabled="false" on both services.

Housekeeping

The PR description still describes this as manifest + startup wiring only. These two commits add a behaviour change to a shared module and a new launch-intent navigation mechanism, and the Testing Instructions don't cover the media-card Open flow — worth updating both, along with a note on which device reproduced the original problem @geekygecko reported.

I did not run ./gradlew spotlessCheck, a build, or the unit tests — finding 1 is from reading the call site, and CI will surface it.

No changes pushed — review feedback only.
• branch feat/tv-media-session

@geekygecko
thanks for the review and nice catch with the media session card!
made some tweaks and now the "Open" action will actually pop the app and navigate to the Now Playing tab!

Claude finished @sztomek's task in 2m 37s —— View job

Code review: TV media-session wiring (round 4)

Reviewed 3a7e061 ("Address PR comments") against the round-3 findings.

• Read the new diff
• Verify the round-3 blocker and the two follow-ups
• Check the new test for correctness / compilation
• Re-check the MediaSessionManager and TvActivity changes
• Post review

All three actionable round-3 items are fixed correctly, and the fixes don't introduce anything new. The remaining open items are the minors I'd already flagged as non-blocking. Nothing left that I'd hold the merge on.

Resolved

1. 🔴 → ✅ :tv:testDebugUnitTest compilation. TvScaffoldViewModelTest.kt:54-58 now passes a real TvLaunchRequests() in the correct positional slot. And you added the test I suggested — openNowPlayingRequests emits when a launch open request is made (line 221) is sound: TvLaunchRequests is in the same package so no import is needed, Channel.CONFLATED buffers the trySend before Turbine's collector runs so there's no ordering race, and no unconsumed events remain at block exit. Worth knowing it asserts the passthrough rather than the outcome — the request → TvTab.NowPlaying step lives in TvScaffold's LaunchedEffect, so it's still only covered by manual testing. Fine given the unit-tests-exemption label.

2. ✅ onCreate re-firing on recreation. savedInstanceState == null guard (TvActivity.kt:20-22) plus setIntent(intent) in onNewIntent (line 32). ComponentActivity always writes a bundle, so every config change and process-death restore now takes the guarded path, and the surviving ViewModel + drained CONFLATED channel mean the re-collecting LaunchedEffect fires nothing spurious. I checked the one case that worried me — a media-card Open landing while the task record survives but the process is dead: ActivityRecord queues pending new intents and delivers them after relaunch, so onNewIntent still runs and the redirect isn't lost. Correct as written.

3. ✅ Cached platform check. private val isTv = Util.isTv(context) at MediaSessionManager.kt:124, immediately after isAutomotive, and both call sites (lines 138, 388) now read the fields. Declaration order is safe — the field initialises before either by lazy block can run.

Still open — all previously flagged, all non-blocking

• The media-card detection heuristic (TvActivity.kt:37-40). Reads as an accepted tradeoff, and your screencast shows it working. Failure mode is silent (a launcher revision switching to CATEGORY_LEANBACK_LAUNCHER just stops the redirect, app still opens), so it degrades safely — but if the follow-up ever comes, the getLaunchActivityPendingIntent(extras) inversion is the version that can't drift.
• Whether singleTask alone fixed Open. Unanswered, and only checkable on the device that reproduced it. Bounded downside: sessionActivity also feeds media3's notification content intent, and nothing in tv/src requests POST_NOTIFICATIONS at runtime, so on API 33+ nothing is displayed anyway.
• Buffered request firing late if the launch lands on Landing / Sign-In, and the tab staying on Now Playing when the queue never reaches Loaded (hasCurrentEpisode starts false, so the corrective collector in TvScaffoldViewModel.init needs a transition it won't get). Both need a stale media card to reach and both degrade to the tv_nothing_playing_* empty state.
• Housekeeping: the PR description now covers the media-card work well. The Testing Instructions still don't mention which device reproduced the original broken Open that @geekygecko reported — useful for whoever revisits the sessionActivity decision.

I did not run ./gradlew spotlessCheck, a build, or the unit tests (no network for dependency resolution here); the diff is formatted consistently with its surroundings and CI covers all three.

No changes pushed — review feedback only.
• branch feat/tv-media-session